Do not show error message for initialization if plugin is already disabled #3536
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR prevents showing the initialization failure error message when a plugin is already disabled; it simply logs the exception.
- Added a conditional check to skip error message display for plugins already marked as disabled
- Retained logging of the initialization exception for disabled plugins
| pair.Metadata.Disabled = true; | ||
| pair.Metadata.HomeDisabled = true; | ||
| failedPlugins.Enqueue(pair); | ||
| if (pair.Metadata.Disabled && pair.Metadata.HomeDisabled) |
There was a problem hiding this comment.
[nitpick] Consider adding a debug log message inside this branch to record that the plugin's initialization was skipped because it is already disabled, which can aid future troubleshooting.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
@Jack251970 can you log debug that it's disabled due to error in init also please.
|
🥷 Code experts: no user but you matched threshold 10 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
This comment has been minimized.
This comment has been minimized.
📝 Walkthrough""" WalkthroughThe change updates the error handling logic in the plugin initialization process. Now, when a plugin fails to initialize, the system checks if the plugin is already marked as disabled. If so, it avoids re-disabling and suppresses repeated error notifications for plugins that are already off. Changes
Sequence Diagram(s)sequenceDiagram
participant PluginManager
participant Plugin
PluginManager->>Plugin: Initialize
alt Initialization succeeds
PluginManager-->>Plugin: Continue normal operation
else Initialization fails
PluginManager->>Plugin: Check Disabled & HomeDisabled flags
alt Plugin already disabled
PluginManager-->>Plugin: Skip error handling
else Plugin not disabled
PluginManager->>Plugin: Set Disabled & HomeDisabled to true
PluginManager->>Plugin: Enqueue to failedPlugins
end
end
Assessment against linked issues
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
📝 WalkthroughWalkthroughThe change updates the plugin initialization error handling in the Changes
Sequence Diagram(s)sequenceDiagram
participant PluginManager
participant Plugin
participant FailedPluginsQueue
PluginManager->>Plugin: Initialize
alt Initialization fails
PluginManager->>Plugin: Check Disabled & HomeDisabled flags
alt Both flags are true
PluginManager-->>Plugin: Skip error handling
else One or both flags are false
PluginManager->>Plugin: Set Disabled & HomeDisabled to true
PluginManager->>FailedPluginsQueue: Enqueue plugin
end
end
Assessment against linked issues
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
When one plugin is already disabled, we do not need to show initialization failure notification and we just need to log exception for it.
Or else the error message will be shown every time Flow starts which can be really annoying.
Resolve #1216.